Conversation
|
The latest push to PR-1674 has stopped. Check out the logs SAN-4781-payments-integration-angular-unit |
|
The latest push to PR-1674 is running on SAN-4781-payments-integration-runnable-angular |
| $scope.helpCards = helpCards; | ||
| $scope.server = {}; | ||
| $scope.activeAccount = $rootScope.dataApp.data.activeAccount; | ||
| $scope.activeAccount = currentOrg.github; // I'm unsure if this is used. |
There was a problem hiding this comment.
You mean our old practice of using stuff attached to the scope from 5 or 6 levels up is not great for determining how and when a variable is used... impossible!
There was a problem hiding this comment.
Something like that.
|
LGTM! Awesome PR that cleans up a bunch of our stuff. |
|
Would it be a lot of work to add a loading screen here?
|
|
Also, seems like I can add modals on top of modals.
|
|
Also, can't dismiss this modal. Is that on purpose? |
|
That's a bug! Good catch. |
|
Turns out almost all of the above issues are related to @thejsj having a localStorage var for hasDismissedTrialNotification set to true, when it's now an object. Since no user has seen this yet it'll get defaulted to an object so it's a non-issue at the moment. There is one issue with a digest cycle required to render the billing page, this is something that I have not yet noticed until now. |
# Conflicts: # client/directives/modals/settingsModal/forms/billingForm/changePaymentForm/changePaymentForm.jade # client/services/serviceFetch.js # test/unit/controllers/controllerApp.unit.js
| !activeAccount.attrs.hasPaymentMethod && !keypather.get($localStorage, 'hasDismissedTrialNotification.' + activeAccount.attrs.id); | ||
| currentOrg.poppa.isInTrial() && | ||
| currentOrg.poppa.trialDaysRemaining() <= 3 && | ||
| !currentOrg.poppa.attrs.hasPaymentMethod && !keypather.get($localStorage, 'hasDismissedTrialNotification.' + currentOrg.github.attrs.id); |
There was a problem hiding this comment.
currentOrg.github.attrs.id should be able to come from poppa
(currentOrg.poppa.attrs.githubId)
There was a problem hiding this comment.
yea, but I have the github object anyways, and if I'm looking for a github ID what better place than from the actual github object we have.
|
lgtm |
4 participants
Fixing issue where we were using properties on the github org instead of big poppa org. Created service to abstract this away.